plugin/base: improvements by gaelicWizard · Pull Request #1930 · Bash-it/bash-it

gaelicWizard · 2021-09-07T05:01:23Z

Description

I've run shellcheck on base.plugin.bash and made a bunch of tweaks to the functions. No behavior changes are intended except the removal of banish-cookies() and command_exists().

Motivation and Context

Just trying to clean up.

How Has This Been Tested?

Tested locally, and all tests pass.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code follows the code style of this project.
If my change requires a change to the documentation, I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
I have added tests to cover my changes, and all the new and existing tests pass.

gaelicWizard · 2021-09-10T00:31:31Z

Is anything else needed for this PR?

NoahGorny

Wow @gaelicWizard, nice cleanup!

I have left a couple of comments. Also I think that this file is full of random stuff that can be moved to some other place, but lets keep this for another PR

plugins/available/base.plugin.bash

gaelicWizard · 2021-09-10T14:26:23Z

I'm leaning more and more towards lots of small files with only a few or even just one function, but that's a whole other PR!

NoahGorny · 2021-09-10T14:42:10Z

I'm leaning more and more towards lots of small files with only a few or even just one function, but that's a whole other PR!

Yap, that should be a part of #1640

NoahGorny

I have only one comment 😄

plugins/available/base.plugin.bash

NoahGorny

Nice job @gaelicWizard !

NoahGorny

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again 😃

gaelicWizard · 2021-09-18T07:27:56Z

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again 😃

One of my goals is to get rid of that file by linting everything! But for the immediate moment, it's kinda hard for me to add things to it since I've got so many open PR's many of which are based on each other and some of which directly change the listing rules, if approved and merged.

NoahGorny · 2021-09-18T09:37:49Z

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again smiley

One of my goals is to get rid of that file by linting everything! But for the immediate moment, it's kinda hard for me to add things to it since I've got so many open PR's many of which are based on each other and some of which directly change the listing rules, if approved and merged.

I dont quite understand, if you add this file here- this makes sure it will always be consistent. If we won't add it, we will need another PR to fix it when we will get rid of clean_files.txt

gaelicWizard · 2021-09-20T20:45:36Z

Rebased on current master and all tests passing. 👍

NoahGorny

Nice work 😄

Quote variables, use $@ and $array[@] instead of $*, typeset some integers, remove unneccesasary binary invocation, use shell features when possible, remove `eval`, &c.

Instead of functions failing when required tools aren't installed, just don't define the function. Alsö, don't redefine del() if it already exists.

Reimplement disk usage function using Bash syntax and simpler layout, without having to invoke an external binary.

New implementation that is even quieter.

Fix `passgen()` to not need `tr`, remove one subshell, and eliminate a useless `echo`.

Adobe Flash is gone with the wind. Alsö, this would be something someone would do *once* and shouldn't be a function...

The `lsgrep()` function is *itself* explicitly forbidden by `shellcheck` rule SC2010. Alsö, s/`$*`/`$@`

Expressly handle unbound parameters.

Newly undisabled `shellcheck` rules

Apply `shfmt` using current project settings. My apologies to future `git blame` hunters. ♥

* master: (113 commits) skip go tests when go is not available plugins: Add ble.sh plugin clean up pyenv plugin Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933) plugin/base: improvements (Bash-it#1930) plugins/percol: `bind` completion/git: `shfmt` && `shellcheck` completion/git: expand search range plugin/percol: `shellcheck` & `shfmt` plugins/percol: use `_command_exists` completion/pip: simplify code flow plugin/less-pretty-cat: remove `|| cat` completion/wpscan: simplify code flow (whitespace) plugins/less-pretty-cat: simplify code flow plugins/less-pretty-cat: use `_command_exists` lib/helpers: cite `_bash-it-find-in-ancestor()` gradle: adopt `_bash_it_find_in_ancestor()` lib/helpers: new function `_bash-it-find-in-ancestor()` completion/laravel: simplify code flow plugin/ruby: add missing parameter error message ...

…autosave-history-plml * 'master' of https://github.com/bash-it/bash-it: (114 commits) ci: Bump go to 1.17 from 1.14 skip go tests when go is not available plugins: Add ble.sh plugin clean up pyenv plugin Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933) plugin/base: improvements (Bash-it#1930) plugins/percol: `bind` completion/git: `shfmt` && `shellcheck` completion/git: expand search range plugin/percol: `shellcheck` & `shfmt` plugins/percol: use `_command_exists` completion/pip: simplify code flow plugin/less-pretty-cat: remove `|| cat` completion/wpscan: simplify code flow (whitespace) plugins/less-pretty-cat: simplify code flow plugins/less-pretty-cat: use `_command_exists` lib/helpers: cite `_bash-it-find-in-ancestor()` gradle: adopt `_bash_it_find_in_ancestor()` lib/helpers: new function `_bash-it-find-in-ancestor()` completion/laravel: simplify code flow ...

* plugins/base: code style improvements Quote variables, use $@ and $array[@] instead of $*, typeset some integers, remove unneccesasary binary invocation, use shell features when possible, remove `eval`, &c. * plugins/base: conditional function definitions Instead of functions failing when required tools aren't installed, just don't define the function. Alsö, don't redefine del() if it already exists. * plugins/base: rewrite `usage()` Reimplement disk usage function using Bash syntax and simpler layout, without having to invoke an external binary. * plugins/base: revamp `quiet()` New implementation that is even quieter. * plugins/base: `myip()` * plugins/base: `pickfrom()` * plugins/base: `passgen()` Fix `passgen()` to not need `tr`, remove one subshell, and eliminate a useless `echo`. * plugins/base: `mkcd()` * plugins/base: `mkiso()` * plugins/base: remove `banish-cookies()` Adobe Flash is gone with the wind. Alsö, this would be something someone would do *once* and shouldn't be a function... * plugins/base: `lsgrep` is SC2010 The `lsgrep()` function is *itself* explicitly forbidden by `shellcheck` rule SC2010. Alsö, s/`$*`/`$@` * plugins/base: `mkiso()` Expressly handle unbound parameters. * plugins/base: remove `command_exists` * plugin/base: lint SC2154 && SC2144 Newly undisabled `shellcheck` rules * plugin/base: import libs for tests * plugin/base: `shfmt` Apply `shfmt` using current project settings. My apologies to future `git blame` hunters. ♥

gaelicWizard force-pushed the plugin-base branch 4 times, most recently from 33d4a07 to 73b59f4 Compare September 9, 2021 01:59

gaelicWizard marked this pull request as ready for review September 9, 2021 01:59

gaelicWizard changed the title ~~DRAFT: Base plugin improvements~~ Base plugin improvements Sep 9, 2021

gaelicWizard force-pushed the plugin-base branch 3 times, most recently from 6f7b2de to 454be11 Compare September 9, 2021 07:31

NoahGorny requested changes Sep 10, 2021

View reviewed changes

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved

gaelicWizard force-pushed the plugin-base branch from 5d5ab32 to 914349e Compare September 10, 2021 18:40

gaelicWizard requested a review from NoahGorny September 10, 2021 18:42

NoahGorny requested changes Sep 11, 2021

View reviewed changes

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved

gaelicWizard force-pushed the plugin-base branch from 914349e to 9998234 Compare September 11, 2021 15:31

gaelicWizard requested a review from NoahGorny September 12, 2021 03:57

gaelicWizard force-pushed the plugin-base branch 6 times, most recently from 73a47e3 to bf9490b Compare September 12, 2021 04:36

NoahGorny approved these changes Sep 12, 2021

View reviewed changes

NoahGorny requested changes Sep 16, 2021

View reviewed changes

gaelicWizard force-pushed the plugin-base branch from bf9490b to 89dd779 Compare September 18, 2021 04:16

gaelicWizard requested a review from NoahGorny September 18, 2021 07:28

gaelicWizard force-pushed the plugin-base branch from b50e1a9 to d312f92 Compare September 20, 2021 20:39

gaelicWizard requested a review from NoahGorny September 20, 2021 20:45

NoahGorny approved these changes Sep 21, 2021

View reviewed changes

gaelicWizard force-pushed the plugin-base branch from d312f92 to 6b9f108 Compare September 24, 2021 05:10

gaelicWizard added 13 commits September 23, 2021 23:00

plugins/base: code style improvements

63869e0

Quote variables, use $@ and $array[@] instead of $*, typeset some integers, remove unneccesasary binary invocation, use shell features when possible, remove `eval`, &c.

plugins/base: conditional function definitions

5cb7522

Instead of functions failing when required tools aren't installed, just don't define the function. Alsö, don't redefine del() if it already exists.

plugins/base: rewrite usage()

c3f45b2

Reimplement disk usage function using Bash syntax and simpler layout, without having to invoke an external binary.

plugins/base: revamp quiet()

41ef92b

New implementation that is even quieter.

plugins/base: myip()

c975f31

plugins/base: pickfrom()

c8ed9a9

plugins/base: passgen()

ea84771

Fix `passgen()` to not need `tr`, remove one subshell, and eliminate a useless `echo`.

plugins/base: mkcd()

9ba76b6

plugins/base: mkiso()

49c9c43

plugins/base: remove banish-cookies()

423729e

Adobe Flash is gone with the wind. Alsö, this would be something someone would do *once* and shouldn't be a function...

plugins/base: lsgrep is SC2010

8d3e81a

The `lsgrep()` function is *itself* explicitly forbidden by `shellcheck` rule SC2010. Alsö, s/`$*`/`$@`

plugins/base: mkiso()

96239a2

Expressly handle unbound parameters.

plugins/base: remove command_exists

280eaf3

gaelicWizard force-pushed the plugin-base branch from 6b9f108 to 9b1485e Compare September 24, 2021 06:16

plugin/base: lint SC2154 && SC2144

bf5670a

Newly undisabled `shellcheck` rules

gaelicWizard force-pushed the plugin-base branch from 9b1485e to ef02917 Compare September 24, 2021 06:18

gaelicWizard added 2 commits September 23, 2021 23:21

plugin/base: import libs for tests

45bf719

plugin/base: shfmt

e66019d

Apply `shfmt` using current project settings. My apologies to future `git blame` hunters. ♥

gaelicWizard force-pushed the plugin-base branch from ef02917 to e66019d Compare September 24, 2021 06:24

NoahGorny merged commit c2c76a3 into Bash-it:master Sep 28, 2021

gaelicWizard deleted the plugin-base branch September 29, 2021 22:59

Conversation

gaelicWizard commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

gaelicWizard commented Sep 10, 2021

Uh oh!

NoahGorny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaelicWizard commented Sep 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NoahGorny commented Sep 10, 2021

Uh oh!

NoahGorny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NoahGorny left a comment

Choose a reason for hiding this comment

Uh oh!

NoahGorny left a comment

Choose a reason for hiding this comment

Uh oh!

gaelicWizard commented Sep 18, 2021

Uh oh!

NoahGorny commented Sep 18, 2021

Uh oh!

gaelicWizard commented Sep 20, 2021

Uh oh!

NoahGorny left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaelicWizard commented Sep 7, 2021 •

edited

Loading

gaelicWizard commented Sep 10, 2021 •

edited

Loading